Skip to content

[18.0][FIX] product_pack: prevent currency being misinterpreted as UoM when printing labels for totalized packs#220

Closed
matiasperalta1 wants to merge 1 commit intoOCA:18.0from
adhoc-dev:18.0-h-98057-mnp
Closed

[18.0][FIX] product_pack: prevent currency being misinterpreted as UoM when printing labels for totalized packs#220
matiasperalta1 wants to merge 1 commit intoOCA:18.0from
adhoc-dev:18.0-h-98057-mnp

Conversation

@matiasperalta1
Copy link

@matiasperalta1 matiasperalta1 commented Aug 13, 2025

Issue:
When printing product labels for pack products with Pack Components Price set to Totalized on the main product, Odoo raised:
AttributeError: 'res.currency' object has no attribute '_compute_quantity'
This happened because in some _get_product_price calls, the currency parameter was being passed as the second positional argument and misinterpreted as uom, causing the crash.

Solution:
Updated _get_product_price to detect when the second positional argument is a res.currency record, move it to kwargs['currency'], and set uom=None. This ensures _compute_price_rule receives the correct parameters and avoids using a currency as a UoM.

Step to reproduce error and test

  1. Create a pack product.
  2. Set Pack Components Price to Totalized.
  3. Add pack component lines.
  4. From the product form, press Print Label. The error should not be appear

@OCA-git-bot
Copy link
Contributor

Hi @pedrobaeza,
some modules you are maintaining are being modified, check this out!

@matiasperalta1 matiasperalta1 changed the title FIX] product_pack: pass currency explicitly in _get_pack_line_price to prevent crash [FIX] product_pack: pass currency explicitly in _get_pack_line_price to prevent crash Aug 13, 2025
@pedrobaeza pedrobaeza changed the title [FIX] product_pack: pass currency explicitly in _get_pack_line_price to prevent crash [18.0][FIX] product_pack: pass currency explicitly in _get_pack_line_price to prevent crash Aug 13, 2025
@pedrobaeza pedrobaeza added this to the 18.0 milestone Aug 13, 2025
@pedrobaeza
Copy link
Member

Which crash? Please check pre-commit.

@matiasperalta1 matiasperalta1 changed the title [18.0][FIX] product_pack: pass currency explicitly in _get_pack_line_price to prevent crash [18.0][FIX] product_pack: prevent currency being misinterpreted as UoM when printing labels for totalized packs Aug 15, 2025
@matiasperalta1
Copy link
Author

@pedrobaeza I have already completed the description of the change

Copy link

@lav-adhoc lav-adhoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pedrobaeza
Copy link
Member

Isn't this a problem of the calling method not using keyword argument?

considering pricelist rules
"""
self and self.ensure_one()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't add empty lines inside a method.

@matiasperalta1
Copy link
Author

@pedrobaeza Yes, the original problem was that the calling method didn't use keyword arguments for currency, causing the currency to be passed as a positional argument and ending up in the uom parameter.
I refactored the call to ensure all relevant arguments were passed as keyword arguments, thus avoiding the error and making the code clearer and safer.

@pedrobaeza
Copy link
Member

Isn't this a problem of the calling method not using keyword argument?

What about my question?

@matiasperalta1
Copy link
Author

@pedrobaeza Yes, exactly. The root cause was the calling method not using keyword arguments. The fix I pushed addresses that by refactoring the call to use explicit keywords.

@pedrobaeza
Copy link
Member

Then this is not correct. You shouldn't fix things at other level that doesn't correspond, as this is complicating this code in an unneeded way. Please fix it where the keyword argument is not being called properly (even at Odoo level).

Imagine if other keyword argument is also not passed as keyword (different from currency_id), or the order is not exactly the same, you will add more code for handling that? I think that's not the proper fix.

I understand it's easier to do it at this level, but think that all the modules will expect the same things in kwargs, and this patch may come more times in other modules, so fix it at the root level.

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be fixed in the root.

@matiasperalta1
Copy link
Author

@pedrobaeza Thanks for the feedback!
I really appreciate your guidance about not patching the issue at the wrong level. I investigated further and addressed the root cause directly in Odoo in this PR: odoo/odoo#223315

@pedrobaeza
Copy link
Member

Fixed in Odoo, so closing.

@pedrobaeza pedrobaeza closed this Aug 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants